feat(webapp): resolve which shard an environment mints run roots into - #4755
Conversation
Adds the third stage of the run-id mint gate chain. `resolveMintShard(env)` returns the shard key an environment mints new roots into: the active shard list, then a per-env or per-org pin, then a rendezvous hash of the environment id. With `RUN_OPS_MINT_SHARDS` unset or empty it returns "new", which is today's behaviour, so this merges inert. `computeRunIdMintKind` and `mintFlipGrace.ts` are untouched. The grace pattern is cloned into `mintShardGrace.ts` rather than widened, so the existing cuid/runOpsId flip grace keeps its behaviour. Design notes: - Pure core plus env-bound wrapper, mirroring `runOpsMintKind.server.ts`. Determinism is a property of `computeMintShard` for fixed deps; the wrapper supplies the clock, exactly as `effectiveMintKind` takes `nowMs`. - Zero new queries on the trigger hot path. Both pins live in the org override blob that `mintRunFriendlyId` already holds. - HRW scores `sha256(envId \0 key)` at 64 bits, over a sorted key list, with a lexicographic tie-break. A 32-bit score collides at our environment count, and without the sort two deployments listing the same keys in a different CSV order would place environments differently. - `parseShardCsv` rejects anything outside [a-z0-9] and rejects the reserved keys at boot. `generateRunOpsIdV2` throws on an out-of-alphabet char, so an unvalidated key would become a throw on the mint path. - A pin outside the active set falls through to the hash and reports once per environment per process. Honouring it would leak the drain the active list performs; throwing would fail customer triggers whenever a pinned shard drains. The loud-on-unknown-key rule governs reading an id, not writing one. - "new" is a legal pin value, holding one org or environment on gen-1 while the rest of the fleet mints gen-2. Without it, a non-empty active set moves every environment at once. - The active-set grace is stamped by `RUN_OPS_MINT_SHARDS_PREV` and `RUN_OPS_MINT_SHARDS_FLIPPED_AT`. A prev list with no timestamp is dropped; a timestamp with an empty prev list graces a first activation. No changeset and no `.server-changes` note: nothing user-visible, and no caller carries the returned key into an id yet.
|
WalkthroughThe change adds gen-2 mint-shard feature flags, validation, pin and override handling, rendezvous-hash assignment, caching, and generation-1 fallbacks. It adds shared grace metadata for mint-kind and shard-set changes. Global flag writes now use transactional stamping, selective persistence, locked-flag validation, and derived-field removal. Admin confirmation lists include stored derived values. Tests cover validation, grace transitions, concurrency, routing, caching, and replacement behavior. Merge Risk: 🟡 Moderate · up to This PR adds shard pinning and routing controls, but malformed or inconsistently validated pin configuration can silently bypass an intended environment or organization pin and place new runs on a different shard. Merge should wait for these bounded routing-correctness risks to be fixed or explicitly accepted by the owner. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… environment A rolling deploy takes hours, so two pods run different values of RUN_OPS_MINT_SHARDS at the same time. The grace window is sized in seconds, so new pods left it long before old pods were gone: for the rest of the rollout the two placed the same environment on different shards. That is the divergence the grace exists to close. The environment variable is now a ceiling that changes only by deploy. It says which shard keys this deployment can mint into. The live list moves to the control-plane database as runOpsMintShardSet, so every pod reads one shared value whatever config generation it is running. Resolution intersects the two, so a stored key this deployment cannot route is never minted into. RUN_OPS_MINT_SHARDS_PREV and RUN_OPS_MINT_SHARDS_FLIPPED_AT are gone. An environment variable cannot record its own flip time, and an operator cannot know a rollout's end in advance. The stamp is now written server-side against the control-plane clock, under an advisory lock, on a genuine change. Stamping generalizes to N graced flag groups in one transaction under one lock, covering the existing mint-kind trio and the new list. That closes a hole on the global admin flags page, which wrote any catalog key with a bare upsert: a graced key could be set with no stamp, or swept away by a save that omitted it. applyGlobalMintKindFlip stays as a thin wrapper so its route and its test keep working unchanged. Operational rule this creates: every change to RUN_OPS_MINT_SHARDS must land across the whole fleet before the flag selects a key it adds. Routing before minting, which is how the shard topology is already gated.
Three areas of the change had no tests. The pure placement logic was well covered; the production entry point and the safety claims were not. resolveMintShard now takes its list reader as a dependency, the same way computeRunIdMintKind takes its flag reader. That makes the cache, the TTL, the ceiling short-circuit and the read fail-safe testable without a database and without mocking. The fail-safe matters: a failed read returns gen-1 rather than guessing a list, because guessing would move every environment's placement for the length of one blip. The catalog tests pin the claim that a bad value is rejected at write. Until now nothing checked it, so an unroutable shard key or a malformed pin blob could have been stored and only failed later. The scope-lock tests pin each key to the scope its resolver reads: pins are locked globally because they are read from the org blob, and the list is locked per-org because it is deployment-wide. Still not covered, and needing a reviewer with Postgres and a browser: the two admin write routes, and boot refusal on a malformed ceiling.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Pins were per-organization and per-environment only, so completing a cutover meant visiting every organization that still carried a canary pin. There was no way to say "every environment mints here now". runOpsMintShardOverride is a global flag that outranks every pin and the hash. Setting it to "new" holds the whole fleet on the current id format, which is the inverse lever for an emergency. It is honored only while the key is in the active list, so it cannot mint into a drained or unroutable shard; an override outside the list is reported and explicit pins still apply. It is read in the same round-trip as the list it is bounded by, so it costs no extra query on the trigger path, and it is locked per-organization because an organization that could override the cutover lever would defeat it. Also marks the ceiling seam: RUN_OPS_MINT_SHARDS is sourced in exactly one place, and it should be deleted once shard descriptors are configured. The descriptors already name every key this deployment can route, so keeping a second hand-maintained list invites the two to drift.
RUN_OPS_MINT_SHARDS was added on this branch and never deployed, so there is nothing to keep compatible. It duplicated information the shard descriptors will own: a descriptor names every key this deployment can route, so a second hand-maintained list only gives the two a way to disagree. Its only job was to stop the list naming a key with no configured database. Nothing here mints, so that cannot happen yet, and by the time it can the descriptors exist and are the right source. Bounding the list belongs with them. The list flag alone is now the gate. Unset or empty means today's behaviour, which is the state of every deployment that has not set it, so this stays inert on merge. env.server.ts is untouched by this branch again. Dependency this creates: the change that carries a shard key into an id must not land before the descriptors bound the list, or it must bound the list itself.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Review found that this branch silently disabled the unset button for runOpsMintKind on the global admin flags page. The graced keys were skipped by the replace sweep so their stamp could not be bare-written, but that skip covered the operator-supplied key as well as the server-computed ones. The page omits a key to unset it, so the omission was read as "leave alone" and the row survived. Before this branch the same gesture deleted it. A graced group is now all-or-nothing. Submitting its primary writes the group with a fresh stamp. Omitting the primary deletes the primary and its stamp together, because a stamp left behind without its primary keeps being served: an empty list beside a live prev list still resolves to the prev list for the rest of the window, which would mint into a shard just removed. Also from review: - The whole save is one transaction again. The stamp, the upserts and the deletes could previously half-apply across two. - The advisory lock takes the previous id as well as the current one, in a fixed order. A deploy rolls for hours, so renaming it left writers on the older release serializing against nothing. Drop the legacy id next release. - A bad global override is reported once per value rather than once per environment. It applies to the whole fleet, so keying the report by environment turned one misconfiguration into a log line and a retained set entry per environment, on the trigger path. Both reporters are bounded now. - The stamp keys render read-only. They were editable controls whose values were discarded on save. - Groups name their primary and derived keys instead of relying on position. - Corrected a claim in a comment: the cache TTL does not bound cross-process disagreement on its own, because the read goes to a replica. Stated why that is tolerable here specifically. - The deprecated single-group entry point is gone; its test now covers the grouped one.
Observability mapAs of Nothing in this pull request moves the report any more. The findings an earlier push reported are gone. The score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
…up fix Running the Postgres suites surfaced two tests asserting opposite things about the same gesture. One was written before groups became all-or-nothing and expected the list to survive a save that omits it, which is the behaviour that made unset a silent no-op. It is replaced with the property that is actually correct: resubmitting the same list alongside another flag leaves the list and its cutover clock alone. Nothing in the implementation changed here. Only a test that encoded the old bug did.
…ion helper Both graced writes called client.$transaction directly. The repo rule is to use the $transaction helper from db.server, which adds the OTEL span and logs the infrastructure errors the raw client swallows. One of these writes stamps a cutover window and the other deletes flags, so a transaction that silently did not run is the case most worth seeing. The helper resolves undefined instead of throwing when it swallows such an error, so both call sites now treat that as a failure the caller sees.
Both global write routes carried their own copy of two answers: which flag keys are graced, and which are server-computed. The JSON API named all four derived keys in a destructure and both primaries in its branch condition. So adding a graced group needed an edit in three files, and missing one would either write an unstamped flip or accept a stamp from a request body. Both answers now come from the group table. touchesGracedGroup and withoutDerivedKeys are exported and used by the route, so a new group needs no route change at all. The global page's managed-cloud refusal moves into lockedFlagsInPayload, a pure function, for the same reason: it encoded the locked-flag policy inline where nothing could test it. That is what closes the coverage gap. The routes previously held branch logic reachable only through an authenticated request, so it went untested while the function underneath it was well covered. The logic is now pure and tested directly, including that only a graced PRIMARY selects the stamped path: a body holding just a stamp must not reset a cutover clock.
…p helpers The previous commit claimed both global write routes derive their graced-key knowledge from the group table. Only one did. The JSON API imported the two helpers and called neither, still naming all four derived keys in a destructure and both primaries in its branch condition. The edit that was supposed to replace that body silently matched nothing, and nothing caught it: unused imports are not type errors, knip checks exports rather than imports within a file, and the repo lint task is broken on its own config so it never ran. The claim went into a commit message unverified. The harm was real rather than cosmetic. A third graced group would have fallen through to the direct write, which sets its stamp from the request body with no lock, which is the failure the helpers exist to prevent. Also from the same review: - The advisory-lock comment had its reasoning inverted. The LEGACY id is the operative one, because an older release takes only that id and is what a rolling deploy has to serialize against. The new id adds nothing until every writer takes it. - Two test comments claimed more than their tests did. The routing tests cover the helpers, not the routes, and cannot show that a route calls them; that is held by review. Said so. - The concurrency test admitted every outcome, so it passed with the lock removed. It now asserts the stored pair is a coherent history: prev is what the other writer left, never the winner's own set. - Merged a duplicate import of the same module.
…flags Two defects this branch introduced. The confirm dialog understated a deletion. Unsetting a graced primary clears its two stamps, and this branch moved those stamps into the locked set, so they left the page's editable keys and the change list stopped mentioning them. Three rows were deleted and one was shown. Before this branch the stamps were editable, so all three appeared. The change list moves into buildFlagChangeList, which adds the cascade. Only an unset cascades: a change re-stamps instead. A stamp that is not stored is not listed. The key topology moves to the shared flag module, since the page and the server both need it and a second copy would drift. The save also wrote every submitted flag. Stamping needs read-then-write, so this branch replaced a batch transaction with an interactive one, where each upsert is its own round trip against the interactive timeout. It now reads the submitted keys once and writes only the values that differ, so a typical save costs two round trips rather than one per flag.
The protected-list case asserted only that the primary survives, so the group-level property was unpinned: protection is keyed off the primary, and the stamps must be kept with it. Third case on this branch of a test claiming more than it asserted.
CI's oxlint flags an unbounded Prisma `in:` filter: the list length becomes the bind-parameter count, so each distinct length is a separate prepared statement. Both lists here are compile-time constants, but boundedIn is what the neighbouring call sites use and it needs no suppression comment. It pads to the next power of two by repeating the last key, which an IN over a unique column does not notice.
…lection-tri-13428 # Conflicts: # apps/webapp/app/routes/admin.feature-flags.tsx # apps/webapp/app/v3/featureFlags.server.ts
The cascade disclosure never fired. buildFlagChangeList looked for the stamps in initialValues, but the page builds initialValues by filtering locked keys out, and the stamps are locked. So the confirm dialog still showed one removal while three rows were deleted. The unit test passed because it was handed an initialValues containing the stamps, which the caller cannot produce. The cascade now reads storedValues, the unfiltered set the loader returned, and the tests use the caller's real shape. Verified against the running app: the dialog lists all three rows with their values.
…lection-tri-13428
…wrapper The placement test reached env.server through its import of the .server module, which the webapp guidance forbids: env.server parses the whole environment schema at import, so the test either fails without a complete environment or passes on ambient values. The file already claimed the core was pure. It now is: mintShardAssignment.ts holds the placement decision and the injected-deps resolver and imports no env, no clock and no database. runOpsMintShard.server.ts keeps only the env-bound wrapper, its caches and its reporters. The test moves next to the pure module and no longer pulls env.server into its chain. knip now sees the wrapper as an unused file rather than an unused export, since nothing imports it until a shard key reaches an id. Ignored by path, with a note to drop the entry with that change.
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/webapp/app/v3/runOpsMigration/mintShardGrace.ts (1)
24-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTie the local flag-key literals to the catalog.
SET_KEY,SET_PREV_KEY, andSET_FLIPPED_AT_KEYrepeat the catalog key names as string literals.runOpsMintShard.server.tsLines 15-20 selects the same rows throughFEATURE_FLAG.runOpsMintShardSetand friends. If a catalog key is renamed, this module keeps the old literal,readStoredCsvreturns[], and every environment silently mints gen-1 with no error.The module comment states the goal is to avoid importing the catalog. A compile-time assertion keeps that property and still fails the rename.
♻️ Proposed guard
Add to
apps/webapp/test/runOpsMintShardFlags.test.ts:it("keeps the pure module's flag keys equal to the catalog keys", () => { expect(FEATURE_FLAG.runOpsMintShardSet).toBe("runOpsMintShardSet"); expect(FEATURE_FLAG.runOpsMintShardSetPrev).toBe("runOpsMintShardSetPrev"); expect(FEATURE_FLAG.runOpsMintShardSetFlippedAt).toBe("runOpsMintShardSetFlippedAt"); });apps/webapp/app/v3/featureFlags.server.ts (1)
189-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBind each stamp function in the group table, not by a fallback ternary.
Line 191 selects
stampMintKindFlipforrunOpsMintKindandstampMintShardSetFlipfor every other group. A third graced group would silently receivestampMintShardSetFlip. That function keys off its ownrunOpsMintShardSetconstants, so it would ignore the new group's primary and stamp shard-set keys instead. The failure is silent: the new group flips with no grace window.The comment on Lines 203-204 states that the group table prevents an unstamped flip. That holds for
touchesGracedGroupbut not for this binding.Declare the stamp function per primary key and fail when a group has none.
♻️ Proposed change
+const STAMPERS: Record<string, typeof stampMintKindFlip> = { + [FEATURE_FLAG.runOpsMintKind]: stampMintKindFlip, + [FEATURE_FLAG.runOpsMintShardSet]: stampMintShardSetFlip, +}; + const GRACED_GLOBAL_GROUPS = GRACED_FLAG_GROUPS.map((group) => { - ...group, - stamp: group.primary === FEATURE_FLAG.runOpsMintKind ? stampMintKindFlip : stampMintShardSetFlip, -})); + const stamp = STAMPERS[group.primary]; + if (!stamp) { + throw new Error(`graced flag group "${group.primary}" has no stamp function`); + } + return { ...group, stamp }; +});
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d2db509b-bdca-43c4-8ff3-9a795ac7ba94
📒 Files selected for processing (16)
apps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/routes/admin.api.v1.feature-flags.tsapps/webapp/app/routes/admin.feature-flags.tsxapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/v3/featureFlags.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.test.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/test/globalFlagChangeList.test.tsapps/webapp/test/globalFlagWriteRouting.test.tsapps/webapp/test/runOpsMintGlobalFlipLock.test.tsapps/webapp/test/runOpsMintShardFlags.test.tsapps/webapp/test/runOpsMintShardSetFlip.test.tsknip.json
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/webapp/test/globalFlagWriteRouting.test.ts
- apps/webapp/test/globalFlagChangeList.test.ts
- apps/webapp/test/runOpsMintGlobalFlipLock.test.ts
- apps/webapp/test/runOpsMintShardSetFlip.test.ts
- knip.json
- apps/webapp/app/v3/runOpsMigration/mintShardGrace.test.ts
- apps/webapp/app/v3/featureFlags.ts
- apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
- apps/webapp/app/routes/admin.api.v1.feature-flags.ts
- apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when:
- Circular dependencies cannot be resolved otherwise
- Code splitting is genuinely needed for performance
- The module must be loaded conditionally at runtime
Files:
apps/webapp/test/runOpsMintShardFlags.test.tsapps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/routes/admin.feature-flags.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/test/runOpsMintShardFlags.test.tsapps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/routes/admin.feature-flags.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/test/runOpsMintShardFlags.test.tsapps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/routes/admin.feature-flags.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: We use vitest exclusively. Never mock anything - use testcontainers instead.
Test files go next to source files (e.g.,MyService.ts->MyService.test.ts).
Files:
apps/webapp/test/runOpsMintShardFlags.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/test/runOpsMintShardFlags.test.tsapps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepath
Files:
apps/webapp/test/runOpsMintShardFlags.test.tsapps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/routes/admin.feature-flags.tsx
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Files:
apps/webapp/test/runOpsMintShardFlags.test.ts
apps/webapp/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Test files must not import
app/env.server.ts; pass configuration as options instead.
Files:
apps/webapp/test/runOpsMintShardFlags.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs. They stay on the
branch throughout development and are stripped byagentcrumbs strip
before merge.
Do not invent new namespaces — pick from this table or ask first.
Do not flag//@Crumbsmarkers, `#region `@crumbsblocks, or agentcrumbs
imports in reviews. These are temporary debug instrumentation stripped
before merge. Data logged in crumbs (IDs, names, values) never reaches
production.
Files:
apps/webapp/test/runOpsMintShardFlags.test.tsapps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/routes/admin.feature-flags.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/routes/admin.feature-flags.tsx
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.ts
apps/webapp/app/v3/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
New code must target Run Engine V2 through the singleton in
app/v3/runEngine.server.ts; do not reintroduce V1 execution paths. V1 branches may only reject or finalize gracefully with a clean 4xx.
Files:
apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.tsapps/webapp/app/v3/featureFlags.server.ts
🧠 Learnings (2)
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/components/admin/flagChangeList.tsapps/webapp/app/v3/runOpsMigration/mintShardGrace.ts
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
apps/webapp/app/v3/featureFlags.server.ts
🪛 ast-grep (0.45.1)
apps/webapp/app/v3/featureFlags.server.ts
[error] 331-350: Recursive/iterative merge copies attacker-controllable keys from a source object into a target via a computed property assignment without rejecting dangerous keys, allowing prototype pollution. Skip or block "proto", "constructor", and "prototype" keys (e.g. if (key === "__proto__" || key === "constructor" || key === "prototype") continue;), use a null-prototype object (Object.create(null)), or use a safe merge utility instead.
Context: for (const key of params.catalogKeys) {
const group = gracedGroupFor(key);
if (group) {
if (requestedFlags[group.primary] !== undefined) {
if (stamped[key] !== undefined) {
toWrite[key] = stamped[key];
}
} else if (!isProtected(group.primary)) {
keysToDelete.push(key);
}
continue;
}
if (key in requestedFlags) {
toWrite[key] = requestedFlags[key];
} else if (!isProtected(key)) {
keysToDelete.push(key);
}
}
Note: [CWE-1321] Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution').
(prototype-pollution-recursive-merge-typescript)
🔇 Additional comments (8)
apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts (2)
1-1: The shard-set read still uses$replica.A previous review asked for the primary client here, and the thread is marked as addressed. Line 1 and Line 27 still use
$replica.applyGlobalGracedFlipswrites the shard-set trio to the primary, so replica lag adds toRUN_OPS_MINT_FLAG_CACHE_TTL_MS. The window in which two pods serve different shard sets becomesTTL + replica lag, which the grace window does not account for.The read runs once per process per TTL, so the primary can absorb it.
🔒 Proposed fix
-import { $replica, boundedIn } from "~/db.server"; +import { boundedIn, prisma } from "~/db.server"; @@ async function readSetFlags(): Promise<Record<string, unknown>> { - const rows = await $replica.featureFlag.findMany({ + const rows = await prisma.featureFlag.findMany({ where: { key: { in: boundedIn(GLOBAL_SHARD_KEYS) } }, select: { key: true, value: true }, });Also applies to: 26-36
41-67: LGTM!Also applies to: 75-92
apps/webapp/app/v3/runOpsMigration/mintShardGrace.ts (1)
28-68: LGTM!Also applies to: 74-99
apps/webapp/test/runOpsMintShardFlags.test.ts (1)
13-118: LGTM!apps/webapp/app/v3/featureFlags.server.ts (2)
331-350: 📐 Maintainability & Code QualityThe prototype-pollution hint is a false positive.
Static analysis flags the computed assignments to
toWrite. The loop iteratesparams.catalogKeys, which the caller derives fromgetAllFlagControlTypes()atapps/webapp/app/routes/admin.feature-flags.tsxLine 134. Those keys are catalog-defined, not request-controlled. No action is needed.Source: Linters/SAST tools
245-248: LGTM!Also applies to: 252-275, 279-296, 307-330, 352-382
apps/webapp/app/routes/admin.feature-flags.tsx (1)
116-121: LGTM!Also applies to: 137-137, 402-402, 469-480, 492-498
apps/webapp/app/components/admin/flagChangeList.ts (1)
16-52: LGTM!
The route-action test mocks ~/db.server to inject its own Prisma client, and that mock exported only prisma and boundedIn. replaceGlobalFeatureFlags now takes the traced $transaction helper from the same module, so all five of the existing cases failed with 'No "$transaction" export is defined'. The stand-in keeps the real semantics under test, an interactive transaction over the injected client, and drops only the tracing and the swallowed-error handling, neither of which this test asserts. Found by running a test file my earlier local runs had skipped. That file arrived with #4751, and picking test files by hand is what hid it.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…al txn helper Two review findings. The cache had a lost-update race. Two misses each issued a read, so a slower read landing after a faster one wrote its older snapshot back into the cache for a whole TTL. Refreshes are now single-flight: concurrent misses await one read, and the in-flight handle is cleared on settle so a failure still lets the next call retry. Three tests cover it with a deferred promise through the injected reader, no mocking. The admin action test's transaction stand-in was a reimplementation. It now delegates to the same shared helper the production wrapper wraps, so the transactional semantics, the nesting case and the retry behaviour are the real ones. Only the wrapper's tracing span and infrastructure-error logging are absent, and neither is asserted there.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts (1)
136-206: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd crumb markers for the changed cache-refresh paths.
Add
//@Crumbsmarkers or `#region `@crumbsblocks while developing these paths. The repository guideline requires crumbs in all changed files.
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts#L136-L206: Mark the single-flight refresh, cache write, and failure fallback paths.apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts#L322-L380: Mark the deferred-read, retry, and cache assertions.As per coding guidelines: “Add crumbs as you write code.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f8a171b5-dffa-423d-b672-8333f9ed46f5
📒 Files selected for processing (3)
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/test/adminFeatureFlagsRouteAction.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (35)
- GitHub Check: report
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: typecheck / typecheck
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: obsmap / 🧪 Unit Tests: Observability Map
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: code-quality / code-quality
- GitHub Check: audit
- GitHub Check: audit
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamic imports. Only use dynamicimport()when:
- Circular dependencies cannot be resolved otherwise
- Code splitting is genuinely needed for performance
- The module must be loaded conditionally at runtime
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: We use vitest exclusively. Never mock anything - use testcontainers instead.
Test files go next to source files (e.g.,MyService.ts->MyService.test.ts).
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepath
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
apps/webapp/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Test files must not import
app/env.server.ts; pass configuration as options instead.
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Add crumbs as you write code — not just when debugging. Mark lines with
//@Crumbsor wrap blocks in `// `#region` `@crumbs. They stay on the
branch throughout development and are stripped byagentcrumbs strip
before merge.
Do not invent new namespaces — pick from this table or ask first.
Do not flag//@Crumbsmarkers, `#region `@crumbsblocks, or agentcrumbs
imports in reviews. These are temporary debug instrumentation stripped
before merge. Data logged in crumbs (IDs, names, values) never reaches
production.
Files:
apps/webapp/test/adminFeatureFlagsRouteAction.test.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
apps/webapp/app/v3/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
New code must target Run Engine V2 through the singleton in
app/v3/runEngine.server.ts; do not reintroduce V1 execution paths. V1 branches may only reject or finalize gracefully with a clean 4xx.
Files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.tsapps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
🧠 Learnings (2)
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts
📚 Learning: 2026-08-24T12:38:01.585Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 4755
File: apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts:1-9
Timestamp: 2026-08-24T12:38:01.585Z
Learning: Keep mint-shard assignment logic in the pure `mintShardAssignment.ts` module so it remains independent of `env.server.ts`, and test that logic in `mintShardAssignment.test.ts` without importing environment-bound modules. Keep runtime and environment integration in the `runOpsMintShard.server.ts` wrapper; do not reintroduce module-level boot warnings tied to the removed `RUN_OPS_MINT_SHARDS` setting.
Applied to files:
apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts
🔇 Additional comments (2)
apps/webapp/test/adminFeatureFlagsRouteAction.test.ts (2)
25-42: Keep this covered by the existing test-mocking finding.This module substitute still mocks
~/db.server. The repository test rule forbids mocks in test files. This concern was already raised on a prior revision.As per coding guidelines,
**/*.{test,spec}.{ts,tsx}tests must not mock anything and must use testcontainers.Source: Coding guidelines
5-5: LGTM!
2 participants
Summary
Adds the shard-selection stage of run-id minting.
resolveMintShard(env)returns which run-ops database an environment mints its new run roots into: the active shard list, then a fleet-wide override, then a per-environment or per-organization pin, then a rendezvous hash of the environment id.That half is inert. Nothing calls
resolveMintShard, no deployment has any of the new flags set, and an empty active list returns the current answer without reading anything.The other half is not inert, and it is where review effort belongs. To stamp a grace window this needs a read-then-write under a lock, so it rewrites the global feature-flag write path that
runOpsMintKindalready depends on in production. See below.Placement
Resolution reads the active list from a global flag, applies the grace window, and then picks:
newholds the whole fleet on the current id format.newholds one organization back while the rest move, which is how a canary works.Two hash details are load-bearing. Scores are 64-bit
sha256(envId \0 key), because a 32-bit score collides at our environment count and an undetected tie would resolve by iteration order. The parsed key list is sorted, because otherwise two deployments listing the same shards in a different CSV order would place environments differently.A pin or override naming a shard that has left the active list falls through to the hash and reports once. Honouring it would leak the drain the active list exists to perform, and throwing would fail triggers whenever a pinned shard drains.
Why the active list is a flag and not an environment variable
A deploy rolls for hours, so two pods hold two different environment values at the same time. A list held in the environment therefore splits the fleet for the length of the rollout, with new pods placing an environment on one shard and old pods on another. A grace window measured in seconds cannot cover that, and the same knob times the existing mint-kind flip so it cannot simply be lengthened. An environment variable also cannot record its own flip time, and an operator cannot know a rollout's end in advance.
So the list, its grace stamp and the override are global flags, written server-side against the control-plane clock under an advisory lock. This branch adds no environment variables.
The write path, which is live
Stamping generalises to any number of graced flag groups in one transaction under one lock. That has three consequences a reviewer should look at directly:
runOpsMintKindis an editable control on the global flags page, and that page previously wrote it with a bare upsert: no lock, no stamp. An operator flipping mint kind through the UI got an ungraced flip, so every pod crossed the cutover at a different moment. Verified against a running instance, before and after.This folds with #4751 rather than replacing it: its
unlockLockedFlagsrule decides what the sweep may delete, and the graced groups keep their stamp under the lock. Both sets of tests pass.Notes for review
Determinism is a property of the pure core for fixed inputs. The wrapper supplies the clock, the same split
effectiveMintKindalready uses. A failed read of the list falls back to the current id format rather than guessing.Six flags appear in the admin pages immediately. The two pins are per-organization, so they render read-only on the global page. The list, its stamp and the override are deployment-wide, so they render read-only in the organization dialog.
Nothing bounds the active list against shards that actually exist. That is safe while nothing mints, but the change that carries a shard key into an id must land after the shard descriptors bound the list, or bound it itself.